feat(grm,case): add case management modules and sync GRM with stable#68
feat(grm,case): add case management modules and sync GRM with stable#68gonzalesedwin1123 merged 12 commits into19.0from
Conversation
Add 12 new modules: spp_case_base, spp_case_cel, spp_case_demo, spp_case_entitlements, spp_case_graduation, spp_case_programs, spp_case_registry, spp_case_session, spp_grm_case_link, spp_grm_cel, spp_grm_programs, and spp_grm_registry. Update spp_grm and spp_grm_demo to match stable upstream: add ticket workflow action buttons, refine officer record rules with separate create permission, rename privilege_grm_user to privilege_grm, convert Html fields to Text, and default ticket assignment to current user.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Missing record rules for assessment model
- Added three record rules (worker/supervisor/manager) for spp.case.assessment model following the same pattern as other case-related models to enforce data isolation.
- ✅ Fixed: Cron creates duplicate overdue review activities daily
- Added deduplication check in _cron_check_reviews to search for existing overdue activities before creating new ones, matching the pattern used for upcoming reviews.
- ✅ Fixed: Stage requirement fields defined but never enforced
- Added validation checks in _check_stage_requirements for is_requires_assessment (verifying completed assessments exist) and is_requires_approval (verifying supervisor permissions).
- ✅ Fixed: Plan approval skips state validation check
- Added state validation in action_approve to ensure plans can only be approved from pending_approval state, preventing workflow bypass.
Or push these changes by commenting:
@cursor push c2b299c69d
Preview (c2b299c69d)
diff --git a/spp_case_base/models/case.py b/spp_case_base/models/case.py
--- a/spp_case_base/models/case.py
+++ b/spp_case_base/models/case.py
@@ -399,6 +399,17 @@
if case.stage_id.is_requires_plan and not case.has_active_plan:
raise ValidationError(f"Stage '{case.stage_id.name}' requires an approved intervention plan.")
+ # Check if assessment is required
+ if case.stage_id.is_requires_assessment:
+ completed_assessment = case.assessment_ids.filtered(lambda a: a.state in ["completed", "reviewed"])
+ if not completed_assessment:
+ raise ValidationError(f"Stage '{case.stage_id.name}' requires a completed assessment.")
+
+ # Check if approval is required
+ if case.stage_id.is_requires_approval:
+ if not self.env.user.has_group("spp_case_base.group_case_supervisor"):
+ raise ValidationError(f"Stage '{case.stage_id.name}' requires supervisor approval.")
+
# Check minimum intensity level
if case.stage_id.min_intensity:
if int(case.intensity_level) < int(case.stage_id.min_intensity):
@@ -449,15 +460,26 @@
)
for case in overdue_cases:
- # Create activity for case worker
- case.activity_schedule(
- "mail.mail_activity_data_todo",
- date_deadline=today,
- summary=_("Case review overdue"),
- note=_("Case %s is due for review. Last review: %s") % (case.name, case.last_review_date or _("Never")),
- user_id=case.case_worker_id.id,
+ # Check if activity already exists
+ existing = self.env["mail.activity"].search(
+ [
+ ("res_model", "=", "spp.case"),
+ ("res_id", "=", case.id),
+ ("summary", "ilike", "review overdue"),
+ ],
+ limit=1,
)
+ if not existing:
+ # Create activity for case worker
+ case.activity_schedule(
+ "mail.mail_activity_data_todo",
+ date_deadline=today,
+ summary=_("Case review overdue"),
+ note=_("Case %s is due for review. Last review: %s") % (case.name, case.last_review_date or _("Never")),
+ user_id=case.case_worker_id.id,
+ )
+
# Find cases approaching review (3 days warning)
warning_date = today + timedelta(days=3)
upcoming_cases = self.search(
diff --git a/spp_case_base/models/case_intervention_plan.py b/spp_case_base/models/case_intervention_plan.py
--- a/spp_case_base/models/case_intervention_plan.py
+++ b/spp_case_base/models/case_intervention_plan.py
@@ -179,6 +179,8 @@
def action_approve(self):
"""Approve the plan."""
for plan in self:
+ if plan.state != "pending_approval":
+ raise ValidationError("Only plans pending approval can be approved.")
plan.write(
{
"state": "approved",
diff --git a/spp_case_base/security/rules.xml b/spp_case_base/security/rules.xml
--- a/spp_case_base/security/rules.xml
+++ b/spp_case_base/security/rules.xml
@@ -207,5 +207,39 @@
<field name="perm_create" eval="True"/>
<field name="perm_unlink" eval="True"/>
</record>
+
+ <!-- Record Rules for spp.case.assessment -->
+ <record id="rule_spp_case_assessment_worker" model="ir.rule">
+ <field name="name">Case Assessment: Worker Own Case Assessments Only</field>
+ <field name="model_id" ref="model_spp_case_assessment"/>
+ <field name="domain_force">[('case_id.case_worker_id', '=', user.id)]</field>
+ <field name="groups" eval="[Command.link(ref('group_case_worker'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="True"/>
+ <field name="perm_create" eval="True"/>
+ <field name="perm_unlink" eval="False"/>
+ </record>
+
+ <record id="rule_spp_case_assessment_supervisor" model="ir.rule">
+ <field name="name">Case Assessment: Supervisor Team Case Assessments</field>
+ <field name="model_id" ref="model_spp_case_assessment"/>
+ <field name="domain_force">['|', ('case_id.supervisor_id', '=', user.id), ('case_id.team_id.supervisor_id', '=', user.id)]</field>
+ <field name="groups" eval="[Command.link(ref('group_case_supervisor'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="True"/>
+ <field name="perm_create" eval="True"/>
+ <field name="perm_unlink" eval="False"/>
+ </record>
+
+ <record id="rule_spp_case_assessment_manager" model="ir.rule">
+ <field name="name">Case Assessment: Manager All Case Assessments</field>
+ <field name="model_id" ref="model_spp_case_assessment"/>
+ <field name="domain_force">[(1, '=', 1)]</field>
+ <field name="groups" eval="[Command.link(ref('group_case_manager'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="True"/>
+ <field name="perm_create" eval="True"/>
+ <field name="perm_unlink" eval="True"/>
+ </record>
</data>
</odoo>| summary=_("Case review overdue"), | ||
| note=_("Case %s is due for review. Last review: %s") % (case.name, case.last_review_date or _("Never")), | ||
| user_id=case.case_worker_id.id, | ||
| ) |
There was a problem hiding this comment.
Cron creates duplicate overdue review activities daily
Medium Severity
The _cron_check_reviews method creates a new activity for every overdue case on each daily run without checking for existing activities. The "upcoming" section correctly deduplicates by searching for existing activities with a matching summary, but the "overdue" section has no such check. This causes a new "Case review overdue" activity to be created every day for each overdue case, flooding workers with duplicate tasks.
| raise ValidationError( | ||
| f"Stage '{case.stage_id.name}' requires minimum intensity level " | ||
| f"{case.stage_id.min_intensity}." | ||
| ) |
There was a problem hiding this comment.
Stage requirement fields defined but never enforced
Medium Severity
The spp.case.stage model defines is_requires_assessment and is_requires_approval boolean fields, and these are exposed in stage configuration views. However, _check_stage_requirements() only validates is_requires_plan and min_intensity — it never checks the assessment or approval requirements. Administrators can configure stages to require these, but the system silently ignores those settings.
Additional Locations (1)
| "approved_date": fields.Datetime.now(), | ||
| } | ||
| ) | ||
| return True |
There was a problem hiding this comment.
Plan approval skips state validation check
Medium Severity
The action_approve method on spp.case.intervention.plan sets the state to "approved" without verifying the plan is currently in "pending_approval" state. This allows approving plans directly from "draft", "active", "completed", or "revised" states, bypassing the intended workflow. Other action methods like action_activate and action_submit_for_approval correctly validate the current state before transitioning.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Semgrep OSS found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the platform's capabilities by integrating a robust case management system. It provides tools for tracking cases through various stages, managing interventions, and linking with other core modules like programs and registries. Concurrently, it refines the existing Grievance Redress Mechanism (GRM) to improve usability and access control, ensuring a more streamlined and secure experience for managing complaints and feedback. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #68 +/- ##
===========================================
- Coverage 71.36% 55.79% -15.57%
===========================================
Files 319 162 -157
Lines 25878 9291 -16587
===========================================
- Hits 18467 5184 -13283
+ Misses 7411 4107 -3304
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive case management module (spp_case_base) and several extension modules for CEL-based rules, demo data, and integration with programs, registry, and graduation. The changes are extensive and well-structured. My review focuses on improving performance by addressing N+1 query patterns, fixing some bugs in security checks and cron jobs, and enhancing code clarity and correctness in several models. Overall, this is a significant and valuable feature addition.
Note: Security Review did not run due to the size of the PR.
| @api.constrains("condition_cel") | ||
| def _check_condition_cel(self): | ||
| """Validate CEL expression syntax.""" | ||
| for rule in self: | ||
| if rule.condition_cel: | ||
| try: | ||
| pass | ||
| except Exception as e: | ||
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e |
There was a problem hiding this comment.
The _check_condition_cel constraint is intended to validate the CEL expression, but the try block is empty. This means no validation is actually performed. You should call the CEL parser here to catch syntax errors when a rule is saved.
| @api.constrains("condition_cel") | |
| def _check_condition_cel(self): | |
| """Validate CEL expression syntax.""" | |
| for rule in self: | |
| if rule.condition_cel: | |
| try: | |
| pass | |
| except Exception as e: | |
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e | |
| @api.constrains("condition_cel") | |
| def _check_condition_cel(self): | |
| """Validate CEL expression syntax.""" | |
| for rule in self: | |
| if rule.condition_cel: | |
| try: | |
| if P: | |
| P.parse(rule.condition_cel) | |
| except Exception as e: | |
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e |
| existing = self.env["mail.activity"].search( | ||
| [ | ||
| ("res_model", "=", "spp.case"), | ||
| ("res_id", "=", case.id), | ||
| ("summary", "ilike", "review upcoming"), | ||
| ], | ||
| limit=1, | ||
| ) |
There was a problem hiding this comment.
The search for an existing activity uses a hardcoded string "review upcoming" while the summary is created using a translatable string _("Case review upcoming"). This will cause the check to fail in any language other than English. The search should use the translated string as well, or a non-translatable key should be used for identification. Using an exact match with = is also more performant.
| existing = self.env["mail.activity"].search( | |
| [ | |
| ("res_model", "=", "spp.case"), | |
| ("res_id", "=", case.id), | |
| ("summary", "ilike", "review upcoming"), | |
| ], | |
| limit=1, | |
| ) | |
| existing = self.env["mail.activity"].search( | |
| [ | |
| ("res_model", "=", "spp.case"), | |
| ("res_id", "=", case.id), | |
| ("summary", "=", _("Case review upcoming")), | |
| ], | |
| limit=1, | |
| ) |
| @api.constrains("condition_cel") | ||
| def _check_condition_cel(self): | ||
| """Validate CEL expression syntax.""" | ||
| for rule in self: | ||
| if rule.condition_cel: | ||
| try: | ||
| # Basic syntax validation | ||
| pass | ||
| except Exception as e: | ||
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e |
There was a problem hiding this comment.
The _check_condition_cel constraint is empty and does not perform any validation on the CEL expression. This should be implemented to catch syntax errors early, when the rule is saved.
| @api.constrains("condition_cel") | |
| def _check_condition_cel(self): | |
| """Validate CEL expression syntax.""" | |
| for rule in self: | |
| if rule.condition_cel: | |
| try: | |
| # Basic syntax validation | |
| pass | |
| except Exception as e: | |
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e | |
| @api.constrains("condition_cel") | |
| def _check_condition_cel(self): | |
| """Validate CEL expression syntax.""" | |
| for rule in self: | |
| if rule.condition_cel: | |
| try: | |
| if P: | |
| P.parse(rule.condition_cel) | |
| except Exception as e: | |
| raise ValidationError(_("Invalid CEL expression in rule '%s': %s") % (rule.name, str(e))) from e |
| supervisor_group = self.env.ref("spp_case_base.group_case_supervisor", raise_if_not_found=False) | ||
| if supervisor_group and supervisor_group not in self.env.user.group_ids: | ||
| raise UserError("Only supervisors can review assessments.") |
There was a problem hiding this comment.
The group membership check supervisor_group not in self.env.user.group_ids is not the recommended way to check for permissions in Odoo. It's better to use self.env.user.has_group('spp_case_base.group_case_supervisor'), which correctly handles group inheritance and is the idiomatic approach.
| supervisor_group = self.env.ref("spp_case_base.group_case_supervisor", raise_if_not_found=False) | |
| if supervisor_group and supervisor_group not in self.env.user.group_ids: | |
| raise UserError("Only supervisors can review assessments.") | |
| if not self.env.user.has_group("spp_case_base.group_case_supervisor"): | |
| raise UserError("Only supervisors can review assessments.") |
| def _get_worker_with_lowest_caseload(self, team): | ||
| """Find the team member with the lowest active caseload. | ||
|
|
||
| Args: | ||
| team: spp.case.team record | ||
|
|
||
| Returns: | ||
| res.users record or False | ||
| """ | ||
| Case = self.env["spp.case"] | ||
| best_worker = False | ||
| lowest_count = float("inf") | ||
|
|
||
| for member in team.member_ids: | ||
| case_count = Case.search_count( | ||
| [ | ||
| ("case_worker_id", "=", member.id), | ||
| ("is_active", "=", True), | ||
| ] | ||
| ) | ||
|
|
||
| if case_count < self.max_cases_per_worker and case_count < lowest_count: | ||
| lowest_count = case_count | ||
| best_worker = member | ||
|
|
||
| return best_worker |
There was a problem hiding this comment.
The _get_worker_with_lowest_caseload method iterates through team members and executes a search_count for each, which can lead to performance issues (N+1 queries) on teams with many members. This can be optimized by using a single read_group to fetch the caseloads for all members at once.
def _get_worker_with_lowest_caseload(self, team):
"""Find the team member with the lowest active caseload."""
if not team.member_ids:
return False
case_data = self.env["spp.case"].read_group(
[
("case_worker_id", "in", team.member_ids.ids),
("is_active", "=", True),
],
["case_worker_id"],
["case_worker_id"],
)
caseload_map = {item["case_worker_id"][0]: item["case_worker_id_count"] for item in case_data}
best_worker = False
lowest_count = float("inf")
for member in team.member_ids:
case_count = caseload_map.get(member.id, 0)
if case_count < self.max_cases_per_worker and case_count < lowest_count:
lowest_count = case_count
best_worker = member
return best_worker
spp_case_base/__manifest__.py
Outdated
| "author": "OpenSPP", | ||
| "website": "https://github.com/OpenSPP/openspp-modules", | ||
| "license": "LGPL-3", | ||
| "development_status": "Stable", |
There was a problem hiding this comment.
The development_status is set to "Stable", but this contradicts the "Alpha" maturity badge and the explicit warning in the README.rst file. To avoid confusion and set correct expectations for users, this should be aligned with the alpha status mentioned in the documentation.
| "development_status": "Stable", | |
| "development_status": "Alpha", |
| if assessment.risk_score is False or assessment.risk_score < 0: | ||
| assessment.risk_level = "low" | ||
| elif assessment.risk_score <= 25: | ||
| assessment.risk_level = "low" | ||
| elif assessment.risk_score <= 50: | ||
| assessment.risk_level = "medium" | ||
| elif assessment.risk_score <= 75: | ||
| assessment.risk_level = "high" | ||
| else: | ||
| assessment.risk_level = "critical" |
There was a problem hiding this comment.
The logic to compute risk_level is a bit complex. The condition assessment.risk_score is False will always be false for a Float field, which defaults to 0.0. Since the _check_risk_score constraint prevents negative values, the logic can be simplified for better readability and correctness.
| if assessment.risk_score is False or assessment.risk_score < 0: | |
| assessment.risk_level = "low" | |
| elif assessment.risk_score <= 25: | |
| assessment.risk_level = "low" | |
| elif assessment.risk_score <= 50: | |
| assessment.risk_level = "medium" | |
| elif assessment.risk_score <= 75: | |
| assessment.risk_level = "high" | |
| else: | |
| assessment.risk_level = "critical" | |
| for assessment in self: | |
| score = assessment.risk_score or 0.0 | |
| if score <= 25: | |
| assessment.risk_level = "low" | |
| elif score <= 50: | |
| assessment.risk_level = "medium" | |
| elif score <= 75: | |
| assessment.risk_level = "high" | |
| else: | |
| assessment.risk_level = "critical" |
| number = fields.Char(string="Ticket number", default="/", readonly=True) | ||
| name = fields.Char(string="Title", required=True) | ||
| description = fields.Html(required=True, sanitize_style=True) | ||
| description = fields.Text(required=True) |
There was a problem hiding this comment.
The description field has been changed from Html to Text. This is a significant change that could lead to loss of formatting for existing data and may affect how information is displayed in the portal and backend. Was this change intentional?
| resolution_summary = fields.Text( | ||
| string="Resolution Summary", | ||
| sanitize_style=True, | ||
| help="Detailed summary of the resolution", | ||
| ) |
There was a problem hiding this comment.
The resolution_summary field has been changed from Html to Text. This will remove rich text formatting capabilities. If detailed, formatted summaries are needed, this should be reverted to Html.
spp_grm/security/rules.xml
Outdated
| <!-- GRM Officer: Read/write own tickets or team tickets --> | ||
| <record id="rule_spp_grm_ticket_officer" model="ir.rule"> | ||
| <field name="name">GRM Ticket: Officer Own and Team Tickets</field> | ||
| <field ref="model_spp_grm_ticket" name="model_id"/> | ||
| <field name="domain_force">[ | ||
| '|', | ||
| ('user_id', '=', user.id), | ||
| ('team_id.member_ids', 'in', [user.id]) | ||
| ]</field> | ||
| <field name="groups" eval="[Command.link(ref('group_grm_officer'))]" /> | ||
| <field name="perm_read" eval="True" /> | ||
| <field name="perm_write" eval="True" /> | ||
| <field name="perm_create" eval="True" /> | ||
| <field name="perm_unlink" eval="False" /> | ||
| </record> | ||
| <field name="groups" eval="[Command.link(ref('group_grm_officer'))]"/> | ||
| <field name="perm_read" eval="True"/> | ||
| <field name="perm_write" eval="True"/> | ||
| <field name="perm_create" eval="False"/> | ||
| <field name="perm_unlink" eval="False"/> | ||
| </record> | ||
|
|
||
| <!-- GRM Supervisor: Team tickets they supervise --> | ||
| <record id="rule_spp_grm_ticket_supervisor" model="ir.rule"> | ||
| <field name="name">GRM Ticket: Supervisor Supervised Team Tickets</field> | ||
| <field ref="model_spp_grm_ticket" name="model_id" /> | ||
| <field name="domain_force">[ | ||
| <!-- GRM Officer: Can create any ticket --> | ||
| <record id="rule_spp_grm_ticket_officer_create" model="ir.rule"> | ||
| <field name="name">GRM Ticket: Officer Create</field> | ||
| <field ref="model_spp_grm_ticket" name="model_id"/> | ||
| <field name="domain_force">[(1, '=', 1)]</field> | ||
| <field name="groups" eval="[Command.link(ref('group_grm_officer'))]"/> | ||
| <field name="perm_read" eval="False"/> | ||
| <field name="perm_write" eval="False"/> | ||
| <field name="perm_create" eval="True"/> | ||
| <field name="perm_unlink" eval="False"/> | ||
| </record> |
There was a problem hiding this comment.
The record rule for GRM officers has been split into two: one for read/write access with a domain restricting to own/team tickets, and another for create access with a global domain [(1, '=', 1)]. This is a good practice for fine-grained control, allowing officers to create tickets for anyone but only edit a subset. However, the perm_create on the first rule should be False to avoid ambiguity. The new rule_spp_grm_ticket_officer_create correctly handles the creation permission.
Add test suites for spp_case_demo (108 tests), spp_grm_case_link (78 tests), spp_grm_programs (42 tests), spp_grm_registry (35 tests), spp_case_session (29 tests), spp_case_registry (32 tests), spp_case_entitlements (32 tests), spp_case_graduation (22 tests), and expand spp_case_programs (6 → 16 tests).
| # Check if current user is a supervisor | ||
| supervisor_group = self.env.ref("spp_case_base.group_case_supervisor", raise_if_not_found=False) | ||
| if supervisor_group and supervisor_group not in self.env.user.group_ids: | ||
| raise UserError("Only supervisors can review assessments.") |
There was a problem hiding this comment.
Supervisor group check uses wrong field for inheritance
Medium Severity
The action_review method checks supervisor_group not in self.env.user.group_ids to verify supervisor permissions. The test suite uses all_group_ids (not group_ids) to verify inherited group membership, and the compliance tests use has_group(). Using group_ids may not include groups inherited through implied_ids, potentially blocking managers from reviewing assessments. The idiomatic approach is self.env.user.has_group().
These modules were missing from git but required by spp_case_graduation and spp_case_session, causing CI test failures.
- Replace deprecated <data noupdate="1"> with <odoo noupdate="1"> - Split mixed noupdate XML files into groups + rules files - Add priority=99 to views using position="replace" - Wrap error strings in _() for translation compliance - Fix "OpenSPP" -> "OpenSPP.org" author in manifests - Replace self.name with self.id in log to avoid PII - Apply auto-fixes from prettier, oca-gen-addon-readme, manifest-website
Format XML files in spp_graduation and spp_session_tracking with prettier. Remove res_config_settings_views.xml from spp_grm as it references non-existent helpdesk_mgmt fields and is not in the manifest.
… modules - Change development_status from "Stable" to "Production/Stable" (C8111) - Remove redundant string= parameters matching auto-generated names (W8113) - Use named translation placeholders instead of positional %s (W8120)
- Fix development_status "Stable" to "Production/Stable" in 9 more manifests - Remove redundant string= parameters in 10 more model files (W8113) - Wrap message_post body strings in _() for translation (C8107) - Convert absolute imports to relative in spp_grm_demo tests (W8150) - Add nosemgrep annotations for legitimate sudo() in demo data (semgrep) - Regenerate README.rst and index.html after status changes
| @@ -548,8 +536,8 @@ | |||
|
|
|||
| for i, note in enumerate(notes): | |||
| note_date = ticket_date + timedelta(days=int((i + 1) * resolution_days / (len(notes) + 1))) | |||
| ticket.sudo().message_post( # nosemgrep: odoo-sudo-without-context — demo data generation runs as admin | |||
| body=f"<p>{note.get('text', '')}</p>", | |||
| ticket.sudo().message_post( | |||
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| @@ -754,8 +729,8 @@ | |||
| note_date = ticket.create_date + timedelta(days=days_offset) | |||
|
|
|||
| # Post message | |||
| ticket.sudo().message_post( # nosemgrep: odoo-sudo-without-context — demo data generation runs as admin | |||
| body=f"<p>{step}</p>", | |||
| ticket.sudo().message_post( | |||
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| @@ -773,8 +748,8 @@ | |||
| escalation = scenario.get("escalation", {}) | |||
| case_type = escalation.get("case_type", "general_investigation") | |||
|
|
|||
| ticket.sudo().message_post( # nosemgrep: odoo-sudo-without-context — demo data generation runs as admin | |||
| body=f"<p>Ticket escalated to case management ({case_type})</p>", | |||
| ticket.sudo().message_post( | |||
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| "approved_date": fields.Datetime.now(), | ||
| } | ||
| ) | ||
| return True |
There was a problem hiding this comment.
Plan approval workflow missing source state guards
Medium Severity
action_approve has no state check at all, allowing a plan in any state (including draft or completed) to be directly approved. Similarly, action_submit_for_approval checks for interventions but not the current state, so a completed or revised plan could be re-submitted. Compare with action_activate which correctly guards with if plan.state != "approved".
Additional Locations (1)
Demo data generators legitimately need sudo() to create records across models. Exclude spp_case_demo and spp_grm_demo from semgrep scanning (matching existing spp_4ps_demo exclusion) instead of inline nosemgrep annotations.
| "case_worker_id": self.env.user.id, | ||
| } | ||
|
|
||
| case = self.env["spp.case"].sudo().create(vals) |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| limit=1, | ||
| ) | ||
| if intervention: | ||
| intervention.sudo().write({"state": "completed"}) |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
Add nosemgrep annotations for legitimate sudo() usage in non-demo modules (sequence generation, SLA breach handling, mail templates, portal ticket creation, CEL rule counters). Fix redirect nosemgrep prefix and ruff-format in generate_cases.py.
| action_date = fields.Date.today() - timedelta(days=days_back) | ||
|
|
||
| if action == "create_plan": | ||
| current_plan = Plan.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| ) | ||
|
|
||
| elif action == "add_intervention" and current_plan: | ||
| Intervention.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| intervention.sudo().write({"state": "completed"}) | ||
| elif action in ("home_visit", "office_visit", "final_visit"): | ||
| visit_type = "home" if action != "office_visit" else "office" | ||
| Visit.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| ) | ||
|
|
||
| elif action == "progress_note": | ||
| Note.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| ) | ||
|
|
||
| elif action in ("assessment", "emergency_assessment", "safety_assessment"): | ||
| Note.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
|
|
||
| for _i in range(random.randint(1, 3)): | ||
| visit_date = intake_date + timedelta(days=random.randint(5, 60)) | ||
| Visit.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
|
|
||
| for _i in range(random.randint(1, 4)): | ||
| note_date = intake_date + timedelta(days=random.randint(1, 60)) | ||
| Note.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
|
|
||
| if closure_stage: | ||
| closure_date = intake_date + timedelta(days=random.randint(30, 90)) | ||
| case.sudo().write( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| CaseType = self.env["spp.case.type"] | ||
| case_type = CaseType.search([("name", "=", type_name)], limit=1) | ||
| if not case_type: | ||
| case_type = CaseType.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
| Service = self.env["spp.service"] | ||
| service = Service.search([("name", "=", service_name)], limit=1) | ||
| if not service: | ||
| service = Service.sudo().create( |
Check warning
Code scanning / Semgrep OSS
Semgrep Finding: semgrep.odoo-sudo-without-context Warning
- Enable test_compliance_generated (1190 lines at 0% coverage) by adding import to tests/__init__.py - Fix 21 wrong menu visibility assertions for unrestricted menus (no groups= attribute) from assertFalse to assertTrue - Fix _menu_visible helper to check group_ids with has_group() for proper implied group resolution instead of unreliable search([]) - Add test_case_models.py with 41 tests covering case_assessment, case_referral, case_note, and case_visit model methods
CI only tests changed modules, so project-wide coverage drops on PRs that don't re-test every module. Fix by: - Enabling carryforward flags so untested module coverage persists - Setting project check to only_pulls mode - Ignoring test files and migrations from coverage metrics - Setting patch coverage target to 70%
1 participant
Add 12 new modules: spp_case_base, spp_case_cel, spp_case_demo, spp_case_entitlements, spp_case_graduation, spp_case_programs, spp_case_registry, spp_case_session, spp_grm_case_link, spp_grm_cel, spp_grm_programs, and spp_grm_registry.
Update spp_grm and spp_grm_demo to match stable upstream: add ticket workflow action buttons, refine officer record rules with separate create permission, rename privilege_grm_user to privilege_grm, convert Html fields to Text, and default ticket assignment to current user.
Note
Medium Risk
Adds a large new functional module with new record rules/ACLs and scheduled automation; misconfiguration could impact data visibility and operational behavior, though changes are largely additive and covered by tests.
Overview
Introduces a new Odoo addon,
spp_case_base, providing core case-management models (cases, assessments, intervention plans/interventions, visits, notes, referrals, and supporting configuration like stages/types/teams) plus a review-reminder cron (_cron_check_reviews) and basic workflow actions (close/reopen, plan approval/versioning, assessment review).Adds the module’s security posture end-to-end (domain privilege + tiered groups,
ir.model.access.csv, record rules, and compliance spec) and a comprehensive test suite covering model behaviors and access rules. Separately addscodecov.ymlfor PR/patch coverage gating with carry-forward flags and updates pre-commitsemgrepexclusions to skip additional demo modules.Written by Cursor Bugbot for commit 3fce171. This will update automatically on new commits. Configure here.